Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Visual output of Alias Analysis #581

Merged
merged 1 commit into from
Jan 18, 2024
Merged

Conversation

xeren
Copy link
Collaborator

@xeren xeren commented Dec 1, 2023

Adds the option program.analysis.printAliasGraph for specifying a file to store a graphviz representation of the FieldSensitiveAndersen analysis.

@hernanponcedeleon
Copy link
Owner

I haven't yet checked the code, but some comments from trying to use this.

  • The code only generates the dot file. It would be good to also call dot -Tpng and generate a figure instead.
  • I find it annoying to have to give the full path. I would prefer to automatically generate this in ./output/filename-alias.png. This is more of a personal preference, but it is aligned with every other data we generate.
  • The graph has directed edges, but I assume they represent the fact that both events may point to the same address, and this is undirect information, so we can drop the point of the arrow (I assume this can be done in graphviz).
  • The graphs seem to be quite dense ... maybe we should avoid relating events from different threads but coming from the same line of code which obviously alias.

@ThomasHaas
Copy link
Collaborator

ThomasHaas commented Dec 9, 2023

I agree with the point of merging events of different threads with the same line of code (it looks much easier to read).

I haven't thought about all the properties of the graph, but if the important aliasing edges are transitive, then a transitive reduction + merging of SCCs will help a lot without losing any information at all.
Even if the relation is not transitive, one could still try to detect and merge equivalent nodes.

Also, how come you can construct this graph before running the core algorithm? Is the graph imprecise because you do it early?

@hernanponcedeleon
Copy link
Owner

The figures look better now. However, avoiding events from different threads, but same line of code, seems not to be implemented yet, which make large figures (like qspinlock attached) still unreadable.

client-code-opt-alias.zip

@hernanponcedeleon
Copy link
Owner

@xeren It seems printGraph is never called in AliasAnalysis, thus the figure is not generated no matter which option I set.

@xeren
Copy link
Collaborator Author

xeren commented Jan 11, 2024

The graph has directed edges, but I assume they represent the fact that both events may point to the same address, and this is undirect information, so we can drop the point of the arrow (I assume this can be done in graphviz).

The prior version of the graph had more emphasis on the internals of the analysis: What variables exist and how they interact.
The former graphs were directed dense because they had a more internal view on the analysis: Nodes represented events with a common thread and source location and for each memory event, there are two analysis elements. There is the program order
With the new changes, the generated graph now reflects the results of the analysis. Undirected graphs like this might actually be more suited for users, because they are just projections of the loc relation.

safe_stack-alias

I haven't thought about all the properties of the graph, but if the important aliasing edges are transitive, then a transitive reduction + merging of SCCs will help a lot without losing any information at all.
Even if the relation is not transitive, one could still try to detect and merge equivalent nodes.

same-line ; loc ; same-line is not transitive, but you could do this with the former visualization, since that is supposed to be transitive. I was initially aiming at the state before the main loop of the algorithm, as I expected it was expressive, while also close to the transitive reduction.

@hernanponcedeleon
Copy link
Owner

LGTM.

@xeren can you rebase so I can merge?

@hernanponcedeleon hernanponcedeleon merged commit 1612e26 into development Jan 18, 2024
1 check passed
@hernanponcedeleon hernanponcedeleon deleted the optimize-alias branch January 18, 2024 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants